Skip to content

[Core] Reject path traversal via embedded ".." segments in path_in_repo - #4884

Open
bodapatisaikrishna wants to merge 1 commit into
huggingface:mainfrom
bodapatisaikrishna:fix/path-in-repo-traversal
Open

[Core] Reject path traversal via embedded ".." segments in path_in_repo#4884
bodapatisaikrishna wants to merge 1 commit into
huggingface:mainfrom
bodapatisaikrishna:fix/path-in-repo-traversal

Conversation

@bodapatisaikrishna

@bodapatisaikrishna bodapatisaikrishna commented Sep 11, 2026

Copy link
Copy Markdown

Summary

_validate_path_in_repo() (used by every CommitOperationAdd/Copy/Delete, so
upload_file, upload_folder, create_commit, delete_file, delete_folder, ...)
only rejects a .. segment when it's the first path component:

if path_in_repo == "." or path_in_repo == ".." or path_in_repo.startswith("../"):
    raise ValueError(...)

A .. later in the path still escapes the repo root once resolved, and slips through
unchanged:

>>> from huggingface_hub._commit_api import _validate_path_in_repo
>>> _validate_path_in_repo("a/../../etc/passwd")
'a/../../etc/passwd'   # not rejected, even though it resolves outside the repo root
>>> _validate_path_in_repo("a/b/../../../etc/passwd")
'a/b/../../../etc/passwd'   # same

This fixes it by walking the full path and tracking depth relative to the repo root,
raising as soon as it would go negative (net traversal above the root) — instead of
only checking the very first component. A .. that resolves back into the repo
without escaping (e.g. "a/../file.txt") is still allowed, matching current behavior.

I don't know how the server itself handles a raw .. in a commit path, so I can't say
whether this is exploitable end-to-end today — this closes a gap in the client-side
guard either way, and the function's own comment says its purpose is exactly this
("prevent a server-side issue").

Test plan

Added cases to TestCommitOperationPathInRepo in tests/test_commit_api.py:
invalid_values gets "a/../../file.txt" and "a/b/../../../file.txt"; valid_values
gets "a/../file.txt" and "a/b/../../c/file.txt" to confirm non-escaping .. is
still allowed unchanged.

$ python -m pytest tests/test_commit_api.py tests/test_utils_paths.py -q
28 passed in 0.07s

ruff check/ruff format/ty check clean on the changed files.

Found this by reading _commit_api.py and testing _validate_path_in_repo directly
with a few traversal patterns, not from a reported issue.


Note

Medium Risk
Security-hardening of path validation used by all commit/upload/delete APIs; behavior change only rejects previously accepted malicious paths, with low regression risk for normal paths covered by new tests.

Overview
Tightens client-side path_in_repo validation in _validate_path_in_repo() so commit paths cannot escape the repo root via .. in the middle of a path (e.g. a/../../file.txt), not only when the path starts with ../.

After the existing leading-slash and prefix checks, the validator now walks every path segment and tracks depth from the repo root; it raises ValueError if a .. would drive depth below zero. Paths where .. stays inside the repo (e.g. a/../file.txt) are still accepted unchanged.

Tests in TestCommitOperationPathInRepo add invalid traversal examples and valid in-repo .. cases for add/delete operations.

Reviewed by Cursor Bugbot for commit a1cbea3. Bugbot is set up for automated code reviews on this repo. Configure here.

_validate_path_in_repo() only rejected a ".." segment when it was the first path
component (exact "..", or a "../" prefix). A "../" later in the path, e.g.
"a/../../etc/passwd", still resolves outside the repo root but was left unchanged.

Walk the full path and track depth relative to the repo root, raising as soon as
it would go negative, instead of only checking the first component. A ".." that
resolves back into the repo without escaping (e.g. "a/../file.txt") is still
allowed, matching current behavior.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant